fix: get realpath from __filename instead of __dirname (NS)
#618
Conversation
Without this fix, this plugin doesn't work if your node_modules tree is made up of directories, but your files are symlinks. It's admittedly a weird environment, but that's the environment that the Bazel build system runs code in, which is the build system we use at Dropbox. This is the bug: - index.js is called from the runfiles directory, and it uses the realpath of its directory as the key for a function on the loaderContext object. - loader.js is called from the bazel-bin/npm directory outside of its runfiles (because Webpack escapes the runfiles when it calls loaders). It uses the realpath of its directory as a key on the loaderContext object, but because it's in a different directory from index.js, it can't find the function set on the loaderContext by index.js. The fix is to get the realpath of the filename instead of the directory so that both files point to the same place.
|
|
Codecov Report
@@ Coverage Diff @@
## master #618 +/- ##
=======================================
Coverage 87.41% 87.41%
=======================================
Files 7 7
Lines 302 302
Branches 68 68
=======================================
Hits 264 264
Misses 36 36
Partials 2 2
Continue to review full report at Codecov.
|
michael-ciniawsky
changed the title
__filename instead of __dirname (NS)
|
@samertm Please sign the CLA in any case by closing and reopening the PR to trigger the CLA Bot again |
michael-ciniawsky
changed the title
__filename instead of __dirname (NS)__filename instead of __dirname (NS)
|
@michael-ciniawsky - Pretty benign change, land it as soon as @samertm signs the CLA & i'll get a patch version out to npm. |
|
Checking with Dropbox that I can sign the NDA, I'll get back to you soon! |
|
@samertm - CLA not NDA if that makes a difference. https://js.foundation/CLA It's pretty concise, in summary ... It's your contribution & you understand the OSS license for this repo / org. |
|
Hey! Just a heads up, I probably won't get permission to sign the CLA until next week. |
|
No worries. If it helps we've had other googlers get approved for other repos but like MS I'm sure it's a per repo basis. No rush. |
|
Hey! Sorry for the wait. It seems like the CLA is an individual CLA. Do you all have a corporate CLA that someone at Dropbox could sign and list me as an authorized contributor? Thanks! |
@kborchers Do we have this? |
|
@sokra @samertm We do not have an automated system for corporate CLAs. We did try to work really hard with a number of organizations to ensure that our individual CLA would be acceptable for employees to sign and have many organizations that normally require a corporate CLA allowing their employees to sign the individual CLA. For reference, we currently have no active corporate CLAs with any organization right now across all JS Foundation projects as the individual CLA has been deemed acceptable. That said, if Dropbox does insist on a corporate CLA, we can have legal draft one for approval by Dropbox legal. Once approved, there is then a manual process to add contributors (or remove users if they leave Dropbox) to the approved list by getting a signed addendum from Dropbox each time they want to authorize one or more GitHub accounts to contribute under their corporate CLA. As you can see, it is definitely much easier for everyone if we can use the individual CLA but we are happy to work with Dropbox if that is what they prefer. |
|
@samertm friendly ping |
|
@samertm Could someone take this over in case there is no way for you to sign the CLA atm ? |
michael-ciniawsky
added
pr_action: Blocked
status: Approved
and removed
pr_action: Review
status: Pending
labels
|
Hey! Sorry this is taking so long. I'm pretty sure I'll be able to sign it, but things are moving a bit slow on getting permission to do so. |
|
Hey! I just signed the CLA, sorry about the wait :) |
* See PR here: webpack-contrib#618
* See PR here: webpack-contrib#618
* See PR here: webpack-contrib#618
Without this fix, this plugin doesn't work if your node_modules tree is
made up of real directories, but your files are symlinks. It's admittedly a
weird environment, but that's the environment that the Bazel build
system runs code in, which is the build system we use at Dropbox.
This is the bug:
index.js is called from the runfiles directory, and it uses the
realpath of its directory as the key for a function on the
loaderContext object.
loader.js is called from the bazel-bin/npm directory outside
of its runfiles (because Webpack escapes the runfiles when it
calls loaders). It uses the realpath of its directory as a
key on the loaderContext object, but because it's in a
different directory from index.js, it can't find the function
set on the loaderContext by index.js.
The fix is to get the realpath of the filename instead of the
directory so that both files point to the same place.